-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated TestSteps to add expectation strings and match the descriptions in the current test plan. #33667
Updated TestSteps to add expectation strings and match the descriptions in the current test plan. #33667
Conversation
PR #33667: Size comparison from 20b1457 to 928fe76 Full report (7 builds for mbed, nrfconnect, qpg, stm32)
|
PR #33667: Size comparison from 20b1457 to 4373a69 Full report (77 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
src/python_testing/TC_BOOLCFG_2_1.py
Outdated
TestStep(8, "Read AlarmsEnabled attribute, if supported"), | ||
TestStep(9, "Read AlarmsSupported attribute, if supported"), | ||
TestStep(10, "Read SensorFault attribute, if supported"), | ||
TestStep(1, "{comDutTH}.", "", is_commissioning=True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
things I should have mentioned earlier - things get a bit weird if you pull these directly from the adoc files because all the stuff in the curly braces are adoc macros (er...they don't call them macros, but basically macros).
So this will render correctly in the adoc (I think?), but it'll also show up directly like this in the test harness.
The solution Rene and I were kicking around was using actual python functions to return the text. ex like here: https://github.com/project-chip/connectedhomeip/pull/32182/files#diff-8bafa8e5179589abd80fe1365c30897596b32305372c4ee3986f1062b9afa0fd
The bonus there is you can consolidate all of this nicely by doing ex.
def constraints(min: typing.Union[int, str], max: stepnum: typing.Union[int, str], type: typing.Union[int, str, None):
type_str = '' if type is none else f'dut_reply() a {type} value. '
return f'{type_str}Value has to be between a range of {min} and {max}'
TestStep(3, read_attribute('SupporedSensitivitiyLevels'), constraints('uint8', 2, 10))
aside...man I'd love to get rid of these attribute constraint tests. Maybe in 1.5 we can start picking them off from the easy DM XMLs. This seems like SO much manual copy-paste for no reason.
Anyway @ReneJosefsen - what are your thoughts here? Do we like this style better than what's in the adoc currently? Is this going to be easier for people to keep in sync if we can generate the test plans automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From just an initial view, this seems like a move in the wrong direction (for now). As you say, this does render fine in adoc once the import is there, but the macros/document attributes are not going to be adjusted accordingly here so it will look very weird when running this on TH.
So for this to work we need to have a similar text replacement in place first IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated TC_BOOLCFG_2_2.py
to do text replacement. See here.
If we like this, I will update the rest of the TC_BOOLCFG_*.py
files.
PR #33667: Size comparison from 20b1457 to a4405f1 Full report (77 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
Good catch on step 14, I will get that sorted. It does look like it does not even render properly because it are missing the last column in that row. Update: I have opened https://github.com/CHIP-Specifications/chip-test-plans/pull/4192 |
PR #33667: Size comparison from 20b1457 to ab8f93b Increases above 0.2%:
Increases (41 builds for cc13x4_26x4, cyw30739, esp32, linux, nrfconnect, telink)
Decreases (27 builds for bl602, bl702, bl702l, efr32, linux, psoc6)
Full report (76 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Updated the TestSteps for several of the python tests, as per #240, now that expectations are supported.
Descriptions and expectations now match what's currently in the test plan. I ran each of these tests through the generator script and compared the output to the current test plan to confirm it matches.
Tests updated:
TC_ACE_1_3
TC_ACL_2_2
TC_BOOLCFG_*
Some notes:
TC_BOOLCFG_4_4
step 14. Looks like the expectation is either missing or it got rolled into the description. See here. This should probably be figured out before we merge these changes.src/python_testing/test_plan_support.py
to take the place of the adoc macros. This has currently been done inTC_BOOLCFG_2_2
, but I will update the others if we're happy with this solution.